PopupMenu: redo keynav using St.FocusManager

Each menu is a focus manager group, but there is also some explicit
focus handling between non-hierarchically-related widgets. Eg, to move
between menus, or from a menubutton into its menu.

https://bugzilla.gnome.org/show_bug.cgi?id=621671

diff --git a/js/ui/appDisplay.js b/js/ui/appDisplay.js
index 55d07685eb1d3cb1f13d639c5b23a3846a2a7969..8c3330e1930aa015f02b1e62697eece0fbbe8ced 100644 (file)
--- a/js/ui/appDisplay.js
+++ b/js/ui/appDisplay.js
@@ -501,11 +501,10 @@ AppWellIcon.prototype = {
                 }
             }));
 
-            this._menuManager.addMenu(this._menu, true);
+            this._menuManager.addMenu(this._menu);
         }
 
         this._menu.popup();
-        this._menuManager.grab();
 
         return false;
     },

diff --git a/js/ui/panel.js b/js/ui/panel.js
index d683a1813658abf527827685784f887ecca0d669..da1079c79a4b3d064c5c770eeab8c7c475de7308 100644 (file)
--- a/js/ui/panel.js
+++ b/js/ui/panel.js
@@ -743,8 +743,9 @@ Panel.prototype = {
         /* Translators: If there is no suitable word for "Activities" in your language, you can use the word for "Overview". */
         let label = new St.Label({ text: _("Activities") });
         this.button = new St.Clickable({ name: 'panelActivities',
-                                          style_class: 'panel-button',
-                                          reactive: true });
+                                         style_class: 'panel-button',
+                                         reactive: true,
+                                         can_focus: true });
         this.button.set_child(label);
 
         this._leftBox.add(this.button);

diff --git a/js/ui/panelMenu.js b/js/ui/panelMenu.js
index 029e08e387cd60fad07e789ea3065f1bb15a0f6a..95c203bafd082ae0f2771469bf16faa19d3142b2 100644 (file)
--- a/js/ui/panelMenu.js
+++ b/js/ui/panelMenu.js
@@ -1,5 +1,6 @@
 /* -*- mode: js2; js2-basic-offset: 4; indent-tabs-mode: nil -*- */
 
+const Clutter = imports.gi.Clutter;
 const St = imports.gi.St;
 const Lang = imports.lang;
 const PopupMenu = imports.ui.popupMenu;
@@ -13,11 +14,13 @@ Button.prototype = {
     _init: function(menuAlignment) {
         this.actor = new St.Bin({ style_class: 'panel-button',
                                   reactive: true,
+                                  can_focus: true,
                                   x_fill: true,
                                   y_fill: false,
                                   track_hover: true });
         this.actor._delegate = this;
         this.actor.connect('button-press-event', Lang.bind(this, this._onButtonPress));
+        this.actor.connect('key-press-event', Lang.bind(this, this._onKeyPress));
         this.menu = new PopupMenu.PopupMenu(this.actor, menuAlignment, St.Side.TOP, /* FIXME */ 0);
         this.menu.connect('open-state-changed', Lang.bind(this, this._onOpenStateChanged));
         Main.chrome.addActor(this.menu.actor, { visibleInOverview: true,
@@ -29,10 +32,27 @@ Button.prototype = {
         this.menu.toggle();
     },
 
+    _onKeyPress: function(actor, event) {
+        let symbol = event.get_key_symbol();
+        if (symbol == Clutter.KEY_space || symbol == Clutter.KEY_Return) {
+            this.menu.toggle();
+            return true;
+        } else if (symbol == Clutter.KEY_Down) {
+            if (!this.menu.isOpen)
+                this.menu.toggle();
+            this.menu.activateFirst();
+            return true;
+        } else
+            return false;
+    },
+
     _onOpenStateChanged: function(menu, open) {
-        if (open)
+        if (open) {
             this.actor.add_style_pseudo_class('pressed');
-        else
+            let focus = global.stage.get_key_focus();
+            if (!focus || (focus != this.actor && !menu.contains(focus)))
+                this.actor.grab_key_focus();
+        } else
             this.actor.remove_style_pseudo_class('pressed');
     }
 };

diff --git a/js/ui/popupMenu.js b/js/ui/popupMenu.js
index 56c7013c450c7a4fcbb01c4ebe5836c26ca4b8b1..f572e13549e28cf025d1d95e04de85212864b956 100644 (file)
--- a/js/ui/popupMenu.js
+++ b/js/ui/popupMenu.js
@@ -58,7 +58,8 @@ PopupBaseMenuItem.prototype = {
                                          hover: true });
         this.actor = new Shell.GenericContainer({ style_class: 'popup-menu-item',
                                                   reactive: params.reactive,
-                                                  track_hover: params.reactive });
+                                                  track_hover: params.reactive,
+                                                  can_focus: params.reactive });
         this.actor.connect('get-preferred-width', Lang.bind(this, this._getPreferredWidth));
         this.actor.connect('get-preferred-height', Lang.bind(this, this._getPreferredHeight));
         this.actor.connect('allocate', Lang.bind(this, this._allocate));
@@ -72,19 +73,39 @@ PopupBaseMenuItem.prototype = {
         this.active = false;
 
         if (params.reactive && params.activate) {
-            this.actor.connect('button-release-event', Lang.bind(this, function (actor, event) {
-                this.emit('activate', event);
-            }));
+            this.actor.connect('button-release-event', Lang.bind(this, this._onButtonReleaseEvent));
+            this.actor.connect('key-press-event', Lang.bind(this, this._onKeyPressEvent));
         }
         if (params.reactive && params.hover)
-            this.actor.connect('notify::hover', Lang.bind(this, this._hoverChanged));
+            this.actor.connect('notify::hover', Lang.bind(this, this._onHoverChanged));
+        if (params.reactive)
+            this.actor.connect('key-focus-in', Lang.bind(this, this._onKeyFocusIn));
     },
 
     _onStyleChanged: function (actor) {
         this._spacing = actor.get_theme_node().get_length('spacing');
     },
 
-    _hoverChanged: function (actor) {
+    _onButtonReleaseEvent: function (actor, event) {
+        this.emit('activate', event);
+        return true;
+    },
+
+    _onKeyPressEvent: function (actor, event) {
+        let symbol = event.get_key_symbol();
+
+        if (symbol == Clutter.KEY_space || symbol == Clutter.KEY_Return) {
+            this.emit('activate', event);
+            return true;
+        }
+        return false;
+    },
+
+    _onKeyFocusIn: function (actor) {
+        this.setActive(true);
+    },
+
+    _onHoverChanged: function (actor) {
         this.setActive(actor.hover);
     },
 
@@ -97,9 +118,10 @@ PopupBaseMenuItem.prototype = {
 
         if (activeChanged) {
             this.active = active;
-            if (active)
+            if (active) {
                 this.actor.add_style_pseudo_class('active');
-            else
+                this.actor.grab_key_focus();
+            } else
                 this.actor.remove_style_pseudo_class('active');
             this.emit('active-changed', active);
         }
@@ -110,10 +132,6 @@ PopupBaseMenuItem.prototype = {
         this.emit('destroy');
     },
 
-    handleKeyPress: function(event) {
-        return false;
-    },
-
     // true if non descendant content includes @actor
     contains: function(actor) {
         return false;
@@ -337,6 +355,8 @@ PopupSliderMenuItem.prototype = {
     _init: function(value) {
         PopupBaseMenuItem.prototype._init.call(this, { activate: false });
 
+        this.actor.connect('key-press-event', Lang.bind(this, this._onKeyPressEvent));
+
         if (isNaN(value))
             // Avoid spreading NaNs around
             throw TypeError('The slider value must be a number');
@@ -482,10 +502,10 @@ PopupSliderMenuItem.prototype = {
         return this._value;
     },
 
-    handleKeyPress: function(event) {
+    _onKeyPressEvent: function (actor, event) {
         let key = event.get_key_symbol();
-        if (key == Clutter.Right || key == Clutter.Left) {
-            let delta = key == Clutter.Right ? 0.1 : -0.1;
+        if (key == Clutter.KEY_Right || key == Clutter.KEY_Left) {
+            let delta = key == Clutter.KEY_Right ? 0.1 : -0.1;
             this._value = Math.max(0, Math.min(this._value + delta, 1));
             this._slider.queue_repaint();
             this.emit('value-changed', this._value);
@@ -611,6 +631,14 @@ PopupMenu.prototype = {
         this._boxWrapper.add_actor(this._box);
         this.actor.add_style_class_name('popup-menu');
 
+        global.focus_manager.add_group(this.actor);
+
+        if (sourceActor._delegate instanceof PopupSubMenuMenuItem) {
+            this._isSubMenu = true;
+            this.actor.reactive = true;
+            this.actor.connect('key-press-event', Lang.bind(this, this._onKeyPressEvent));
+        }
+
         this.isOpen = false;
         this._activeMenuItem = null;
     },
@@ -709,12 +737,10 @@ PopupMenu.prototype = {
         }
     },
 
-    open: function(submenu) {
+    open: function() {
         if (this.isOpen)
             return;
 
-        this.emit('opening');
-
         let primary = global.get_primary_monitor();
 
         // We need to show it now to force an allocation,
@@ -730,7 +756,7 @@ PopupMenu.prototype = {
         let menuWidth = natWidth, menuHeight = natHeight;
 
         // Position the non-pointing axis
-        if (submenu) {
+        if (this._isSubmenu) {
             if (this._arrowSide == St.Side.TOP || this._arrowSide == St.Side.BOTTOM) {
                 // vertical submenu
                 if (sourceY + sourceHeigth + menuHeight + this._gap < primary.y + primary.height)
@@ -763,8 +789,6 @@ PopupMenu.prototype = {
         if (!this.isOpen)
             return;
 
-        this.emit('closing');
-
         if (this._activeMenuItem)
             this._activeMenuItem.setActive(false);
         this.actor.reactive = false;
@@ -773,7 +797,6 @@ PopupMenu.prototype = {
         this.emit('open-state-changed', false);
     },
 
-
     toggle: function() {
         if (this.isOpen)
             this.close();
@@ -781,35 +804,15 @@ PopupMenu.prototype = {
             this.open();
     },
 
-    handleKeyPress: function(event, submenu) {
-        if (!this.isOpen || (submenu && !this._activeMenuItem))
-            return false;
-        if (this._activeMenuItem && this._activeMenuItem.handleKeyPress(event))
-            return true;
-        switch (event.get_key_symbol()) {
-        case Clutter.space:
-        case Clutter.Return:
-            if (this._activeMenuItem)
-                this._activeMenuItem.activate(event);
+    _onKeyPressEvent: function(actor, event) {
+        // Move focus back to parent menu if the user types Left.
+        // (This handler is only connected if the PopupMenu is a
+        // submenu.)
+        if (this.isOpen &&
+            this._activeMenuItem &&
+            event.get_key_symbol() == Clutter.KEY_Left) {
+            this._activeMenuItem.setActive(false);
             return true;
-        case Clutter.Down:
-        case Clutter.Up:
-            let items = this._box.get_children().filter(function (child) { return child.visible && child.reactive; });
-            let current = this._activeMenuItem ? this._activeMenuItem.actor : null;
-            let direction = event.get_key_symbol() == Clutter.Down ? 1 : -1;
-
-            let next = findNextInCycle(items, current, direction);
-            if (next) {
-                next._delegate.setActive(true);
-                return true;
-            }
-            break;
-        case Clutter.Left:
-            if (submenu) {
-                this._activeMenuItem.setActive(false);
-                return true;
-            }
-            break;
         }
 
         return false;
@@ -844,6 +847,7 @@ PopupSubMenuMenuItem.prototype = {
     _init: function(text) {
         PopupBaseMenuItem.prototype._init.call(this, { activate: false, hover: false });
         this.actor.connect('enter-event', Lang.bind(this, this._mouseEnter));
+        this.actor.connect('key-press-event', Lang.bind(this, this._onKeyPressEvent));
 
         this.label = new St.Label({ text: text });
         this.addActor(this.label);
@@ -888,7 +892,7 @@ PopupSubMenuMenuItem.prototype = {
     setActive: function(active) {
         if (this.menu) {
             if (active)
-                this.menu.open(true);
+                this.menu.open();
             else
                 this.menu.close();
         }
@@ -896,14 +900,14 @@ PopupSubMenuMenuItem.prototype = {
         PopupBaseMenuItem.prototype.setActive.call(this, active);
     },
 
-    handleKeyPress: function(event) {
+    _onKeyPressEvent: function(actor, event) {
         if (!this.menu)
             return false;
-        if (event.get_key_symbol() == Clutter.Right) {
+        if (event.get_key_symbol() == Clutter.KEY_Right) {
             this.menu.activateFirst();
             return true;
         }
-        return this.menu.handleKeyPress(event, true);
+        return false;
     },
 
     contains: function(actor) {
@@ -929,6 +933,7 @@ PopupMenuManager.prototype = {
         this.grabbed = false;
 
         this._eventCaptureId = 0;
+        this._keyPressEventId = 0;
         this._enterEventId = 0;
         this._leaveEventId = 0;
         this._activeMenu = null;
@@ -936,21 +941,20 @@ PopupMenuManager.prototype = {
         this._delayedMenus = [];
     },
 
-    addMenu: function(menu, noGrab, position) {
+    addMenu: function(menu, position) {
         let menudata = {
             menu:              menu,
             openStateChangeId: menu.connect('open-state-changed', Lang.bind(this, this._onMenuOpenState)),
             activateId:        menu.connect('activate', Lang.bind(this, this._onMenuActivated)),
             destroyId:         menu.connect('destroy', Lang.bind(this, this._onMenuDestroy)),
             enterId:           0,
-            buttonPressId:     0
+            focusId:           0
         };
 
         let source = menu.sourceActor;
         if (source) {
-            menudata.enterId = source.connect('enter-event', Lang.bind(this, this._onMenuSourceEnter, menu));
-            if (!noGrab)
-                menudata.buttonPressId = source.connect('button-press-event', Lang.bind(this, this._onMenuSourcePress, menu));
+            menudata.enterId = source.connect('enter-event', Lang.bind(this, function() { this._onMenuSourceEnter(menu); }));
+            menudata.focusId = source.connect('key-focus-in', Lang.bind(this, function() { this._onMenuSourceEnter(menu); }));
         }
 
         if (position == undefined)
@@ -974,8 +978,8 @@ PopupMenuManager.prototype = {
 
         if (menudata.enterId)
             menu.sourceActor.disconnect(menudata.enterId);
-        if (menudata.buttonPressId)
-            menu.sourceActor.disconnect(menudata.buttonPressId);
+        if (menudata.focusId)
+            menu.sourceActor.disconnect(menudata.focusId);
 
         this._menus.splice(position, 1);
     },
@@ -984,6 +988,7 @@ PopupMenuManager.prototype = {
         Main.pushModal(this._owner.actor);
 
         this._eventCaptureId = global.stage.connect('captured-event', Lang.bind(this, this._onEventCapture));
+        this._keyPressEventId = global.stage.connect('key-press-event', Lang.bind(this, this._onKeyPressEvent));
         // captured-event doesn't see enter/leave events
         this._enterEventId = global.stage.connect('enter-event', Lang.bind(this, this._onEventCapture));
         this._leaveEventId = global.stage.connect('leave-event', Lang.bind(this, this._onEventCapture));
@@ -994,24 +999,30 @@ PopupMenuManager.prototype = {
     ungrab: function() {
         global.stage.disconnect(this._eventCaptureId);
         this._eventCaptureId = 0;
+        global.stage.disconnect(this._keyPressEventId);
+        this._keyPressEventId = 0;
         global.stage.disconnect(this._enterEventId);
         this._enterEventId = 0;
         global.stage.disconnect(this._leaveEventId);
         this._leaveEventId = 0;
 
-        Main.popModal(this._owner.actor);
-
         this.grabbed = false;
+        Main.popModal(this._owner.actor);
     },
 
     _onMenuOpenState: function(menu, open) {
-        if (!open && menu == this._activeMenu)
-            this._activeMenu = null;
-        else if (open)
+        if (open) {
             this._activeMenu = menu;
+            if (!this.grabbed)
+                this.grab();
+        } else if (menu == this._activeMenu) {
+            this._activeMenu = null;
+            if (this.grabbed)
+                this.ungrab();
+        }
     },
 
-    _onMenuSourceEnter: function(actor, event, menu) {
+    _onMenuSourceEnter: function(menu) {
         if (!this.grabbed || menu == this._activeMenu)
             return false;
 
@@ -1021,13 +1032,6 @@ PopupMenuManager.prototype = {
         return false;
     },
 
-    _onMenuSourcePress: function(actor, event, menu) {
-        if (this.grabbed)
-            return false;
-        this.grab();
-        return false;
-    },
-
     _onMenuActivated: function(menu, item) {
         if (this.grabbed)
             this.ungrab();
@@ -1084,33 +1088,39 @@ PopupMenuManager.prototype = {
                    || (eventType == Clutter.EventType.KEY_PRESS && event.get_key_symbol() == Clutter.Escape)) {
             this._closeMenu();
             return true;
-        } else if (eventType == Clutter.EventType.KEY_PRESS
-                   && this._activeMenu != null
-                   && this._activeMenu.handleKeyPress(event, false)) {
-                return true;
-        } else if (eventType == Clutter.EventType.KEY_PRESS
-                   && this._activeMenu != null
-                   && (event.get_key_symbol() == Clutter.Left
-                       || event.get_key_symbol() == Clutter.Right)) {
-            let direction = event.get_key_symbol() == Clutter.Right ? 1 : -1;
+        } else if (activeMenuContains || this._eventIsOnAnyMenuSource(event)) {
+            return false;
+        }
+
+        return true;
+    },
+
+    _onKeyPressEvent: function(actor, event) {
+        if (!this.grabbed || !this._activeMenu)
+            return false;
+        if (!this._eventIsOnActiveMenu(event))
+            return false;
+
+        let symbol = event.get_key_symbol();
+        if (symbol == Clutter.Left || symbol == Clutter.Right) {
+            let direction = symbol == Clutter.Right ? 1 : -1;
             let pos = this._findMenu(this._activeMenu);
             let next = this._menus[mod(pos + direction, this._menus.length)].menu;
             if (next != this._activeMenu) {
-                this._activeMenu.close();
-                next.open(false);
+                let oldMenu = this._activeMenu;
+                this._activeMenu = next;
+                oldMenu.close();
+                next.open();
                 next.activateFirst();
             }
             return true;
-        } else if (activeMenuContains || this._eventIsOnAnyMenuSource(event)) {
-            return false;
         }
 
-        return true;
+        return false;
     },
 
     _closeMenu: function() {
         if (this._activeMenu != null)
             this._activeMenu.close();
-        this.ungrab();
     }
 };